Allow plugin settings to add connect-src CSP sources - #7166
Conversation
Gykes
left a comment
Gykes
left a comment
There was a problem hiding this comment.
I will preface this by saying I don't have a massive amount of knowledge in this area. Feel free to push back and defend any decisions you made.
- Reject commas in connect-src URLs to prevent CSP header breakage - Change Debugf to Warnf for invalid csp_ plugin settings - Add csp-settings opt-in flag to prevent namespace collisions: plugins must set csp-settings: true in their ui section to enable the csp_ setting prefix for dynamic connect-src sources
…l header - Only call GetPluginConfiguration when the plugin sets csp-settings: true, instead of reading it for every enabled plugin on every page request. - Sort the validated URLs so the emitted connect-src is stable between requests (settings is a map). - Warn once per invalid setting value instead of on every page request. - Replace the tautological opt-in test with one that calls setPageSecurityHeaders and asserts on the emitted CSP header. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Pushed One thing worth stating explicitly rather than leaving for a reviewer to find, since it is a deliberate design choice:
The trade-off: anything able to call The stricter alternative is an explicit allowlist in the Happy to switch to the allowlist if you prefer the tighter contract, it is a small change. Flagging the choice rather than making it silently. |
2 participants
Description
Adds a mechanism for plugins to contribute validated
connect-srcentries to Stash's page Content Security Policy via plugin settings, rather than only through the staticui.csp.connect-srcin the pluginyml.Any plugin setting whose key starts with
csp_and whose value is a valid concretehttp/httpsURL is appended to the page'sconnect-srcdirective (per plugin, only for enabled plugins). This lets plugins that talk to a user-configurable backend ship a narrow default and have users set their exact backend endpoint in the plugin settings UI, applied automatically on the next page load — no hand-editing of plugin files, and the value survives plugin updates because it's stored in Stash's config rather than the shipped files.Currently the static
ui.csp.connect-srcforces plugins with user-configurable backends to either ship a broad wildcard (e.g.http://*:7860) — poor security policy — or require users to edit the pluginyml, which is overwritten on every update.Implementation details:
internal/api/server.go: new pure helpercspConnectSrcFromSettings(settings)returns validated connect-src URLs plus keys skipped as invalid, andisValidConnectSrcURL(s)validates strictly vianet/url. InsetPageSecurityHeaders, inside the existing per-plugin loop,GetPluginConfiguration(plugin.ID)is read and validatedcsp_*values are appended toconnect-src. Invalid values are skipped with a debug log (key + plugin ID).internal/api/server_test.go: table-driven tests covering valid URLs, disallowed schemes, non-URLs, non-string values, wildcard hosts, CSP directive-breakout attempts (whitespace/;/quotes), degenerate hosts, userinfo, empty strings, and mixed valid/invalid.Validation intentionally rejects: anything other than concrete
http/httpsURLs with a host, wildcards (http://*:7860), non-string values, empty strings, values containing CSP meta-characters, degenerate hosts, and URLs with userinfo — so a misconfigured or hostile setting can never corrupt or disable the CSP header.pkg/pluginis unchanged; onlyconnect-srcis affected (notscript-src/style-src). No reload or restart is needed — the header is rebuilt per request.Related Issue
Closes #7165
Testing
Unit tests:
go test ./internal/api/ -run TestCspConnectSrcFromSettings— 14/14 pass. Fullgo build ./...succeeds.Manual verification (local instance, plugin with
settings: csp_apiEndpoint):csp_setting.Content-Security-Policyconnect-srcheader initially excludes the setting's URL.https://api.example.comvia theconfigurePluginGraphQL mutation — the URL appeared in the servedconnect-srcheader on the next request, without reload or restart.https://evil.com/; script-src 'none') — it was rejected, and the header remained valid and unchanged.http://*:7860) — rejected, not added to the header.config.ymlunder the plugin ID.Screenshots
No UI changes; this is a server-side CSP header change, so there are no screenshots.
Checklist
AI Usage Disclosure
I used an AI coding assistant (OpenCode with an Deepseek v4 Flash) to help generate and refine the implementation and the accompanying tests. I have reviewed and understood every line and design decision, performed the manual testing described above, and take full responsibility for the change and its AGPL licensing.
Additional Context
This addresses a design gap where plugins with user-configurable backends cannot express per-user connect-src entries, forcing either broad wildcards or manual
ymledits that are lost on plugin update. A follow-up would be for affected plugins (e.g. a face-recognition userscript) to adopt this mechanism and drop their wildcard entries.